Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Issue #506] Allow linking between series #587

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

andrepapoti
Copy link

Description

Allow linking between two series.
A new field called related_series was added to the Series model where you can reference other Series. This feature is can be used to link different versions of the same Series to easily build a version history.
The linking of a Series is bidirectional so if you link A to B automatically B is linked to A.
For this feature a new API endpoint was created so the API version was bumped to v1.4.

Related

Closes #506

Copy link
Member

@stephenfin stephenfin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the delays getting this. I'll try to respond faster to future revisions. I have some minor nits in the docs but my bigger question is this: should this be a dedicated API, or could we allow this as part of a new update series API? For example:

PATCH /api/v1.4/series/1 HTTP/1.1
Content-Type: text/json

{
    "related": [2, 3]
}

This is what we do for patches now, so I wonder if there's any reason not to do the same for series?

(fwiw, I have a clear preference for sticking to RESTful APIs where possible, so I'm wondering what reason, if any, there is for not doing things that way 🙂)

@@ -275,6 +275,7 @@ Supported Versions
1.1, 2.1, ✓
1.2, 2.2, ✓
1.3, 3.1, ✓
1.4, 3.1, ✓
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1.4, 3.1, ✓
1.4, 3.2, ✓

Comment on lines 1 to 2
API v1.3
=================
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
API v1.3
=================
API v1.3
========

@stephenfin
Copy link
Member

Also, we probably want to expose events for this? However, we should be careful not to avoid the pitfall encountered in related patches, namely that we returned the related_patches attribute as it was, despite that fact this would change over time as patches were added/removed.

For two series to be linked togheter they must belong to the same project

Closes getpatchwork#506

Signed-off-by: andrepapoti <[email protected]>
…eries'

Ensure the retrival of series is keept at O(1) complexity.

Closes getpatchwork#506

Signed-off-by: andrepapoti <[email protected]>
@andrepapoti
Copy link
Author

@stephenfin updated the API to follow your suggestion.

My initial idea was to not require the user to just send the new values instead of appending them to the previous related_series in the request's body, but considering that the Patch already follows the format you suggested it and it's not an issue for the clients to get the current related_series of a Series

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow linking to older versions of a series
2 participants